fix: preserve commit error handling through describe/exec/describe#309
Open
hjotha wants to merge 2 commits intomendixlabs:mainfrom
Open
fix: preserve commit error handling through describe/exec/describe#309hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha wants to merge 2 commits intomendixlabs:mainfrom
Conversation
`commit $X on error rollback;` could roundtrip as plain `commit $X;` after rewriting an MPR. The writer omitted `ErrorHandlingType` when it equaled rollback, treating it as implicit. The parser then defaulted an absent field to an empty value, so the next describe had no rollback setting to print. Make parser and writer behavior symmetric: default absent commit `ErrorHandlingType` to rollback while parsing, and always write commit `ErrorHandlingType`, defaulting to rollback. Tests cover parser and writer preservation for commit rollback handling.
AI Code ReviewReview SummaryCritical IssuesNone found. Moderate IssuesNone found. Minor IssuesNone found. What Looks Good
RecommendationApprove. The fix correctly addresses the root cause with appropriate test coverage and maintains backward compatibility. The changes are minimal, well-scoped, and follow the project's patterns for BSON handling. No further action needed. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
Adds an MDL script under mdl-examples/bug-tests/ that exercises describe → exec → describe fixpoint for `commit $X on error rollback;`, plus a negative control using `on error continue;`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AI Code ReviewCritical IssuesNone found. Moderate IssuesNone found. Minor IssuesNone found. What Looks Good
RecommendationApprove the PR. The changes are correct, well-tested, and address the specific issue without introducing side effects. The fix ensures commit error handling (rollback/continue) persists through describe/exec/describe cycles as required. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
36 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #332.
Fixes #308.
Summary
commit $X on error rollback;could roundtrip as plaincommit $X;after rewriting an MPR.Root cause
The commit writer omitted
ErrorHandlingTypewhen it was rollback, treating it as implicit. The parser then left absentErrorHandlingTypeempty, so the describer had no rollback setting to print on the next describe.Fix
Make commit parser/writer behavior symmetric:
ErrorHandlingTypeto rollback.ErrorHandlingType, defaulting to rollback.Tests
Added parser/writer regression coverage for commit rollback preservation.
Validation
make buildmake testmake lint-gomake test-integrationAgentic Code Testing
Test plan